-
Notifications
You must be signed in to change notification settings - Fork 7
SDK-2713-net-sdk-request-header #491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds HTTP response header access functionality to the Yoti .NET SDK, enabling clients to retrieve headers including X-Request-ID for troubleshooting API connection issues while maintaining backward compatibility.
- Introduces
YotiHttpResponse<T>wrapper class to expose both response data and HTTP headers - Modifies existing API methods to return header information alongside response data
- Maintains backward compatibility through implicit conversion operators
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Yoti.Auth/Web/YotiHttpResponse.cs | New wrapper class providing access to HTTP response headers and data |
| src/Yoti.Auth/Web/Request.cs | Adds ExecuteWithHeaders method to support header extraction |
| src/Yoti.Auth/DigitalIdentity/DigitalIdentityService.cs | Implements header-aware versions of share session and receipt methods |
| src/Yoti.Auth/DigitalIdentityClient.cs | Updates client methods to return YotiHttpResponse wrapper |
| src/Yoti.Auth/DocScan/DocScanService.cs | Updates DocScan service methods to return headers |
| src/Yoti.Auth/DocScan/DocScanClient.cs | Updates DocScan client methods to return YotiHttpResponse wrapper |
| test/Yoti.Auth.Tests/Web/YotiHttpResponseTests.cs | Comprehensive test suite for the new YotiHttpResponse functionality |
| src/Yoti.Auth/Examples/RequestHeaderExample.cs | Example code demonstrating header access patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| public async Task<GetSessionResult> GetSession(string sdkId, AsymmetricCipherKeyPair keyPair, string sessionId) | ||
| }).ConfigureAwait(false); | ||
| } public async Task<YotiHttpResponse<GetSessionResult>> GetSession(string sdkId, AsymmetricCipherKeyPair keyPair, string sessionId) |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing line break between method declarations. There should be a proper line break between the closing brace of CreateSession and the start of GetSession method.
| .WithHttpMethod(HttpMethod.Post) | ||
| .WithBaseUri(ApiUri) | ||
| .WithEndpoint("/sessions") | ||
| .WithEndpoint("sessions") |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint path should start with a forward slash. This change from '/sessions' to 'sessions' may cause incorrect URL construction.
| .WithEndpoint("sessions") | |
| .WithEndpoint("/sessions") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mehmet-yoti this should be resolved
| } | ||
|
|
||
| /// <summary> | ||
| } /// <summary> |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing line break between method declarations. There should be a proper line break between the closing brace and the start of the comment block.
| /// Gets a share receipt and returns the result with HTTP response headers | ||
| /// </summary> | ||
| public static async Task<YotiHttpResponse<SharedReceiptResponse>> GetShareReceiptWithHeaders(HttpClient httpClient, string clientSdkId, Uri apiUrl, AsymmetricCipherKeyPair key, string receiptId) | ||
| { | ||
| // For now, call the regular method and wrap the result with empty headers | ||
| // This is a simplified implementation - full header support would require modifying all the internal HTTP calls | ||
| var result = await GetShareReceipt(httpClient, clientSdkId, apiUrl, key, receiptId); | ||
| return YotiHttpResponse<SharedReceiptResponse>.FromHttpResponse(result, new HttpResponseMessage()); |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating an empty HttpResponseMessage loses all header information. This implementation doesn't actually provide headers as promised by the method name. The method should be refactored to properly capture and return HTTP response headers.
| /// Gets a share receipt and returns the result with HTTP response headers | |
| /// </summary> | |
| public static async Task<YotiHttpResponse<SharedReceiptResponse>> GetShareReceiptWithHeaders(HttpClient httpClient, string clientSdkId, Uri apiUrl, AsymmetricCipherKeyPair key, string receiptId) | |
| { | |
| // For now, call the regular method and wrap the result with empty headers | |
| // This is a simplified implementation - full header support would require modifying all the internal HTTP calls | |
| var result = await GetShareReceipt(httpClient, clientSdkId, apiUrl, key, receiptId); | |
| return YotiHttpResponse<SharedReceiptResponse>.FromHttpResponse(result, new HttpResponseMessage()); | |
| /// Gets a share receipt and returns the result and the HTTP response message | |
| /// </summary> | |
| private static async Task<(SharedReceiptResponse, HttpResponseMessage)> GetShareReceiptWithHttpResponse(HttpClient httpClient, string clientSdkId, Uri apiUrl, AsymmetricCipherKeyPair key, string receiptId) | |
| { | |
| Validation.NotNull(httpClient, nameof(httpClient)); | |
| Validation.NotNull(apiUrl, nameof(apiUrl)); | |
| Validation.NotNull(clientSdkId, nameof(clientSdkId)); | |
| Validation.NotNull(key, nameof(key)); | |
| string endpoint = string.Format(shareReceiptRetrieval, receiptId); | |
| Request shareReceiptRequest = new RequestBuilder() | |
| .WithKeyPair(key) | |
| .WithBaseUri(apiUrl) | |
| .WithHeader(yotiAuthId, clientSdkId) | |
| .WithEndpoint(endpoint) | |
| .WithHttpMethod(HttpMethod.Get) | |
| .Build(); | |
| HttpResponseMessage response = await shareReceiptRequest.Execute(httpClient).ConfigureAwait(false); | |
| try | |
| { | |
| if (!response.IsSuccessStatusCode) | |
| { | |
| Response.CreateYotiExceptionFromStatusCode<DigitalIdentityException>(response); | |
| } | |
| var responseObject = await response.Content.ReadAsStringAsync(); | |
| var deserialized = await Task.Factory.StartNew(() => JsonConvert.DeserializeObject<SharedReceiptResponse>(responseObject)); | |
| return (deserialized, response); | |
| } | |
| catch | |
| { | |
| response.Dispose(); | |
| throw; | |
| } | |
| } | |
| /// <summary> | |
| /// Gets a share receipt and returns the result with HTTP response headers | |
| /// </summary> | |
| public static async Task<YotiHttpResponse<SharedReceiptResponse>> GetShareReceiptWithHeaders(HttpClient httpClient, string clientSdkId, Uri apiUrl, AsymmetricCipherKeyPair key, string receiptId) | |
| { | |
| // Call the overload that returns both the result and the HttpResponseMessage | |
| var (result, responseMessage) = await GetShareReceiptWithHttpResponse(httpClient, clientSdkId, apiUrl, key, receiptId); | |
| return YotiHttpResponse<SharedReceiptResponse>.FromHttpResponse(result, responseMessage); |
This PR adds the ability to access HTTP response headers (including X-Request-ID) from API calls in the Yoti .NET SDK, addressing client troubleshooting needs for connection timeout issues while maintaining full backward compatibility.